Skip to content

added sns support and workaround for route53 api limit #16

Merged
merged 3 commits into from
Mar 8, 2022

Conversation

cho00013
Copy link
Contributor

@cho00013 cho00013 commented Mar 7, 2022

  1. Added Environment variable
  • MAX_API_RETRY = maximum retry for r53 api (note - the wait in between increases .. wait 1 sec, then wait 2 sec, etc.)
  • SNS_TOPIC_ARN = ARN of the SNS Topic that should be used as target
  • SNS_ENABLE = control whether SNS should be sent (see sample code that publishes msg)
  1. Removed this line -- from lib2to3.pgen2.pgen import DFAState
  2. Added LOGGER debug level as Environment variable passed in.
  3. Added "def get_sns_client()" and "def publish_to_sns()" for handling SNS publishing
  4. Removed artificiaul wait time. And changed the initial wait from 10, 20 seconds to 5 seconds. This is probably not needed.
  5. Added expotential wait 1, 2, 3, etc seconds up to MAX_API_RETRY times - a value of 10 means up to 55 seconds . These are baked into the 4 main Route 53 API subroutines:
  • new_list_hosted_zones
  • new_get_hosted-zone_properties
  • new_delete_resource_record
  • new_get_resource_record
  • new_change_resource_recordset

NOTE: In my testing (terminating instance), I was able to get 50 simultaneous without an issue. The highest WAIT retry observed was 4 (meaning, it waited 4 times which is 10 seconds) so with the SDK wait of 5 times, that means that the particular instance retried at least 20 (5x4) due to API limit. I think we should be able to support and handle much higher than 50 instances.

  1. Added instance id to be included in the log for better troubleshooting.
  2. Added delete_records variable to track deletion of ALL Route53 records correctly. If one failed, then retain the DDB item in case we need the meta-data to delete things properly.
  3. Switched some LOGGER.info to LOGGER.error.
  4. Modified the code to better log and reuse (e.g. check out variable change_batch).
  5. Added SNS publish API calls for Record Delete and Creation Fails. We can add SQS down the road, if we want to capture the failed DNS record into a SQS (and we can have separate lambda that processes the failed DNS changes). I do include the change_set info in the message.

Code /TF changes required

  1. SNS topic should be created via TF and the Arn passed in as Argument into the Lambda Env variable.
  2. The Lambda Role requires perms to publish Sns (see sample code below)
{
    "Sid": "sns",
    "Effect": "Allow",
    "Action": {
        "sns:Get*",
        "sns:Publish"
    "Resource": <Arn of the Sns topic created>
}

Env Variable for the Lambda function - add:

  • MAX_API_RETRY : use 10 as the initial value.
  • SNS_TOPIC_ARN : see SNS topic above
  • SNS_ENABLE : initially set to False unless you want to test the SNS topic.

@cho00013
Copy link
Contributor Author

cho00013 commented Mar 7, 2022

Added a missing carriage return at the end of the file and incremented version to 0.1.20

code/ddns-lambda.py Outdated Show resolved Hide resolved
code/ddns-lambda.py Outdated Show resolved Hide resolved
code/ddns-lambda.py Outdated Show resolved Hide resolved
code/ddns-lambda.py Outdated Show resolved Hide resolved
Copy link
Contributor

@badra001 badra001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cho00013 Is this code ready to go (other than the couple of minor changes)?

@cho00013
Copy link
Contributor Author

cho00013 commented Mar 7, 2022

@cho00013 Is this code ready to go (other than the couple of minor changes)?

yeah should be good to go. If you want to test the SNS, don't forget to add the SNS topic resource via TF and adjust the IAM role permission for the Lambda.

@cho00013
Copy link
Contributor Author

cho00013 commented Mar 8, 2022

@cho00013 Is this code ready to go (other than the couple of minor changes)?

yeah should be good to go. If you want to test the SNS, don't forget to add the SNS topic resource via TF and adjust the IAM role permission for the Lambda.

Let me address the SNS topic suggestions you have.. Give me few mins to add to the merge request

@badra001
Copy link
Contributor

badra001 commented Mar 8, 2022

Can I add something to the SNS policy to allow the lambda to use it, or do I have to add to the lambda role policy to access the SNS?

  - SNS code prep
  - SNS resource prep
  - refactor the route53 API calls
  - add better API timeouts
  - new variables:
    - sns_topic_name
    - sqs_queue_name
    - enable_sns
    - enable_sqs
@badra001 badra001 merged commit 47b6950 into master Mar 8, 2022
@badra001 badra001 deleted the awspeter_workaround_api_limit branch March 8, 2022 14:34
@cho00013
Copy link
Contributor Author

cho00013 commented Mar 8, 2022

Can I add something to the SNS policy to allow the lambda to use it, or do I have to add to the lambda role policy to access the SNS?

Prob can do it via SNS resource policy, but it's easier and more manageable to do it via IAM Role Permission attached the Lambda. The resource policy is usually used to cross-account access as well as doing it grant to AWS service directly and also doing Deny statement with Condition.

I would recommend modifying the Lambda Role IAM permission.

@badra001
Copy link
Contributor

badra001 commented Mar 8, 2022

Can I add something to the SNS policy to allow the lambda to use it, or do I have to add to the lambda role policy to access the SNS?

Prob can do it via SNS resource policy, but it's easier and more manageable to do it via IAM Role Permission attached the Lambda. The resource policy is usually used to cross-account access as well as doing it grant to AWS service directly and also doing Deny statement with Condition.

I would recommend modifying the Lambda Role IAM permission.

Ok, that is how I set it up (if enable_sns is used for the TF variable).

@cho00013
Copy link
Contributor Author

cho00013 commented Mar 8, 2022

Can I add something to the SNS policy to allow the lambda to use it, or do I have to add to the lambda role policy to access the SNS?

Prob can do it via SNS resource policy, but it's easier and more manageable to do it via IAM Role Permission attached the Lambda. The resource policy is usually used to cross-account access as well as doing it grant to AWS service directly and also doing Deny statement with Condition.
I would recommend modifying the Lambda Role IAM permission.

Ok, that is how I set it up (if enable_sns is used for the TF variable).

I did check your SNS Resource policy and it looks good - looks the same as default one. It appears that you need to grant perms to both Resource policy AND the IAM principal. So we'll need to add the Lambda Role Permission.

Something like this... adding the Sid to the existing inline policy attached to the Lambda ROle.

   {
        "Sid": "SnsPublish",
        "Effect": "Allow",
        "Action": { 
                    "sns:Get*",
                    "sns:publish"
        },
        "Resource": [aws_sns_topic.topic.arn]
    },

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants